Add ordering and ordering_field to AIGeneratedTableSettings for enhanced table sorting#1527
Conversation
…ced table sorting
There was a problem hiding this comment.
Pull request overview
This PR enhances the AI-generated table settings by adding support for default table sorting configuration. The AI service now generates ordering (ASC/DESC) and ordering_field (column name) properties for each table, allowing tables to have sensible default sort orders.
Changes:
- Added
orderingandordering_fieldfields to theAIGeneratedTableSettingsinterface - Updated the AI prompt to instruct the model to generate appropriate sorting configuration
- Implemented validation and mapping logic to process AI-generated sorting values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| settings.readonly_fields = this.filterValidColumns(tableSettings.readonly_fields, validColumnNames); | ||
| settings.columns_view = this.filterValidColumns(tableSettings.columns_view, validColumnNames); | ||
| settings.ordering = this.mapOrdering(tableSettings.ordering); | ||
| settings.ordering_field = validColumnNames.includes(tableSettings.ordering_field) ? tableSettings.ordering_field : null; |
There was a problem hiding this comment.
The validation logic doesn't handle the case when tableSettings.ordering_field is undefined or null. If the AI response is missing this field or it's null, validColumnNames.includes(undefined) or validColumnNames.includes(null) will be called, which will always return false but is not semantically clear. Consider adding an explicit check for undefined/null before the includes check.
| settings.ordering_field = validColumnNames.includes(tableSettings.ordering_field) ? tableSettings.ordering_field : null; | |
| const orderingField = tableSettings.ordering_field; | |
| settings.ordering_field = | |
| orderingField != null && validColumnNames.includes(orderingField) ? orderingField : null; |
| settings.ordering = this.mapOrdering(tableSettings.ordering); | ||
| settings.ordering_field = validColumnNames.includes(tableSettings.ordering_field) ? tableSettings.ordering_field : null; |
There was a problem hiding this comment.
There's no validation to ensure that ordering and ordering_field are set together. If the AI provides an ordering direction but an invalid ordering_field (or vice versa), the table settings will have incomplete sorting configuration. Consider validating that if one is set, the other should also be set, or set both to null if either is invalid to maintain consistency.
| settings.ordering = this.mapOrdering(tableSettings.ordering); | |
| settings.ordering_field = validColumnNames.includes(tableSettings.ordering_field) ? tableSettings.ordering_field : null; | |
| const mappedOrdering = this.mapOrdering(tableSettings.ordering); | |
| const validOrderingField = validColumnNames.includes(tableSettings.ordering_field) | |
| ? tableSettings.ordering_field | |
| : null; | |
| if (mappedOrdering && validOrderingField) { | |
| settings.ordering = mappedOrdering; | |
| settings.ordering_field = validOrderingField; | |
| } else { | |
| settings.ordering = null; | |
| settings.ordering_field = null; | |
| } |
No description provided.